Request changes

Thanks for the ExtensionMap refactor — the overall approach is clean, and the empty-string fix addresses a real issue (BaseModelWithConfig's str_min_length=1 was rejecting valid extension values).

However, during my local review, I found a couple of blocking issues in the ExtensionMap configuration that should be addressed before this PR can be merged.

🔴 Blocking

1. src/ralph/models/xapi/base/common.py — coerce_numbers_to_str=True should be removed

Current implementation:

class ExtensionMap(RootModel[Dict[IRI, Union[str, int, bool, list, dict, None]]]):
    model_config = ConfigDict(str_min_length=0, coerce_numbers_to_str=True)

The fix only needs to allow empty strings; str_min_length=0 is sufficient.

coerce_numbers_to_str=True is unnecessary because the model already explicitly accepts Union[str, int, bool, ...]. Enabling this option also introduces a data-fidelity risk for an LRS by silently coercing numeric values to strings.

Suggested fix:

class ExtensionMap(RootModel[Dict[IRI, Union[str, int, bool, list, dict, None]]]):
    model_config = ConfigDict(str_min_length=0)

2. src/ralph/models/xapi/base/common.py — str_min_length=0 should only apply to extension values

Current implementation:

model_config = ConfigDict(str_min_length=0, ...)

This configuration applies to every string in the model, including IRI keys.

Although empty IRI keys are currently rejected by the IRI validator, the relaxed string constraint should be scoped only to extension values rather than being applied globally.

Suggested fix:

from typing import Annotated
from pydantic import StringConstraints

ExtensionValue = Union[
    Annotated[str, StringConstraints(min_length=0)],
    int,
    bool,
    list,
    dict,
    None,
]

class ExtensionMap(RootModel[Dict[IRI, ExtensionValue]]):
    """Pydantic custom data type for XAPI extensions."""

🟡 Non-blocking

• Remove the duplicated test case in the parametrized tests (None appears twice, lines 145–146).

• Replace the try / except / pytest.fail() pattern with direct model instantiation.

• Consider ordering the union as Union[str, bool, int, ...] to avoid the bool/int ambiguity.

• Add a few negative test cases, for example:

  • empty IRI key ({"": 42});
  • unsupported extension value type.

✅ Local verification

Verified locally:

• On main, an empty string ("") is rejected because BaseModelWithConfig enforces str_min_length=1.
• With this implementation, empty string extension values are accepted as expected.
• The extension test suite passes successfully (12 passed).

Once the blocking issues above are addressed, please rebase this branch onto the current main (which already includes the CI fixes and PRs #630 and #632) before requesting another review.